Do not cache matching results across events in receive()

This should fix #1222.

Akinori MUSHA 9 years ago
parent
commit
28c4f9d14f
2 changed files with 69 additions and 48 deletions
  1. 44 45
      app/models/agents/event_formatting_agent.rb
  2. 25 3
      spec/models/agents/event_formatting_agent_spec.rb

+ 44 - 45
app/models/agents/event_formatting_agent.rb

@@ -95,8 +95,6 @@ module Agents
95 95
       ]
96 96
     end
97 97
 
98
-    after_save :clear_matchers
99
-
100 98
     def validate_options
101 99
       errors.add(:base, "instructions and mode need to be present.") unless options['instructions'].present? && options['mode'].present?
102 100
 
@@ -120,12 +118,15 @@ module Agents
120 118
     end
121 119
 
122 120
     def receive(incoming_events)
121
+      matchers = compiled_matchers
122
+
123 123
       incoming_events.each do |event|
124 124
         interpolate_with(event) do
125
-          interpolation_context.merge(perform_matching(event.payload))
126
-          formatted_event = interpolated['mode'].to_s == "merge" ? event.payload.dup : {}
127
-          formatted_event.merge! interpolated['instructions']
128
-          create_event :payload => formatted_event
125
+          apply_compiled_matchers(matchers, event) do
126
+            formatted_event = interpolated['mode'].to_s == "merge" ? event.payload.dup : {}
127
+            formatted_event.merge! interpolated['instructions']
128
+            create_event payload: formatted_event
129
+          end
129 130
         end
130 131
       end
131 132
     end
@@ -164,49 +165,47 @@ module Agents
164 165
       end
165 166
     end
166 167
 
167
-    def perform_matching(payload)
168
-      matchers.inject(payload.dup) { |hash, matcher|
169
-        matcher[hash]
170
-      }
168
+    def compiled_matchers
169
+      if matchers = options['matchers']
170
+        matchers.map { |matcher|
171
+          regexp, path, to = matcher.values_at(*%w[regexp path to])
172
+          [Regexp.new(regexp), path, to]
173
+        }
174
+      end
171 175
     end
172 176
 
173
-    def matchers
174
-      @matchers ||=
175
-        if matchers = options['matchers']
176
-          matchers.map { |matcher|
177
-            regexp, path, to = matcher.values_at(*%w[regexp path to])
178
-            re = Regexp.new(regexp)
179
-            proc { |hash|
180
-              mhash = {}
181
-              value = interpolate_string(path, hash)
182
-              if value.is_a?(String) && (m = re.match(value))
183
-                m.to_a.each_with_index { |s, i|
184
-                  mhash[i.to_s] = s
185
-                }
186
-                m.names.each do |name|
187
-                  mhash[name] = m[name]
188
-                end if m.respond_to?(:names)
189
-              end
190
-              if to
191
-                case value = hash[to]
192
-                when Hash
193
-                  value.update(mhash)
194
-                else
195
-                  hash[to] = mhash
196
-                end
197
-              else
198
-                hash.update(mhash)
199
-              end
200
-              hash
201
-            }
202
-          }
203
-        else
204
-          []
177
+    def apply_compiled_matchers(matchers, event, &block)
178
+      return yield if matchers.nil?
179
+
180
+      # event.payload.dup does not work; HashWithIndifferentAccess is
181
+      # a source of trouble here.
182
+      hash = {}.update(event.payload)
183
+
184
+      matchers.each do |re, path, to|
185
+        m = re.match(interpolate_string(path, hash)) or next
186
+
187
+        mhash =
188
+          if to
189
+            case value = hash[to]
190
+            when Hash
191
+              value
192
+            else
193
+              hash[to] = {}
194
+            end
195
+          else
196
+            hash
197
+          end
198
+
199
+        m.size.times do |i|
200
+          mhash[i.to_s] = m[i]
205 201
         end
206
-    end
207 202
 
208
-    def clear_matchers
209
-      @matchers = nil
203
+        m.names.each do |name|
204
+          mhash[name] = m[name]
205
+        end
206
+      end
207
+
208
+      interpolate_with(hash, &block)
210 209
     end
211 210
   end
212 211
 end

+ 25 - 3
spec/models/agents/event_formatting_agent_spec.rb

@@ -8,6 +8,7 @@ describe Agents::EventFormattingAgent do
8 8
             :instructions => {
9 9
                 :message => "Received {{content.text}} from {{content.name}} .",
10 10
                 :subject => "Weather looks like {{conditions}} according to the forecast at {{pretty_date.time}}",
11
+                :timezone => "{{timezone}}",
11 12
                 :agent => "{{agent.type}}",
12 13
                 :created_at => "{{created_at}}",
13 14
                 :created_at_iso => "{{created_at | date:'%FT%T%:z'}}",
@@ -19,6 +20,10 @@ describe Agents::EventFormattingAgent do
19 20
                     :regexp => "\\A(?<time>\\d\\d:\\d\\d [AP]M [A-Z]+)",
20 21
                     :to => "pretty_date",
21 22
                 },
23
+                {
24
+                    :path => "{{pretty_date.time}}",
25
+                    :regexp => "(?<timezone>[A-Z]+)\\z",
26
+                },
22 27
             ],
23 28
         }
24 29
     }
@@ -50,8 +55,8 @@ describe Agents::EventFormattingAgent do
50 55
             :name => "somevalue2",
51 56
         },
52 57
         :date => {
53
-            :epoch => "1357966800",
54
-            :pretty => "00:00 AM EST on January 12, 2015"
58
+            :epoch => "1366372800",
59
+            :pretty => "08:00 AM EDT on April 19, 2013"
55 60
         },
56 61
         :conditions => "someothervalue2"
57 62
     }
@@ -85,7 +90,24 @@ describe Agents::EventFormattingAgent do
85 90
       formatted_event1, formatted_event2 = Event.last(2)
86 91
 
87 92
       expect(formatted_event1.payload[:subject]).to eq("Weather looks like someothervalue according to the forecast at 10:00 PM EST")
88
-      expect(formatted_event2.payload[:subject]).to eq("Weather looks like someothervalue2 according to the forecast at 00:00 AM EST")
93
+      expect(formatted_event1.payload[:timezone]).to eq("EST")
94
+      expect(formatted_event2.payload[:subject]).to eq("Weather looks like someothervalue2 according to the forecast at 08:00 AM EDT")
95
+      expect(formatted_event2.payload[:timezone]).to eq("EDT")
96
+    end
97
+
98
+    it "should not fail if no matchers are defined" do
99
+      @checker.options.delete(:matchers)
100
+
101
+      expect {
102
+        @checker.receive([@event, @event2])
103
+      }.to change { Event.count }.by(2)
104
+
105
+      formatted_event1, formatted_event2 = Event.last(2)
106
+
107
+      expect(formatted_event1.payload[:subject]).to eq("Weather looks like someothervalue according to the forecast at ")
108
+      expect(formatted_event1.payload[:timezone]).to eq("")
109
+      expect(formatted_event2.payload[:subject]).to eq("Weather looks like someothervalue2 according to the forecast at ")
110
+      expect(formatted_event2.payload[:timezone]).to eq("")
89 111
     end
90 112
 
91 113
     it "should allow escaping" do